-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added new capability in ice_forcing.F90 to test with JRA55 atmospheri… #350
Conversation
…c forcing on gx1 grid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Just a few questions.
It looks like a test was removed from the base suite, was this intended?
The "if" section around line 1380 of ice_forcing.F90 looks a little odd. Is this correct mixing logic for atm_data_type and atm_data_format?
if (trim(atm_data_type) == 'hadgem') then ! netcdf i = index(data_file,'.nc') - 5 tmpname = data_file write(data_file,'(a,i4.4,a)') tmpname(1:i), yr, '.nc' elseif (trim(atm_data_format) == 'nc') then ! netcdf i = index(data_file,'.nc') - 5 tmpname = data_file write(data_file,'(a,i4.4,a)') tmpname(1:i), yr, '.nc' else ! LANL/NCAR naming convention i = index(data_file,'.dat') - 5 tmpname = data_file write(data_file,'(a,i4.4,a)') tmpname(1:i), yr, '.dat' endif
Have the new data files been put on ncar ftp so others can test? Have the data files been tested from a formal CICE input data area? I don't think I see the input files on conrad in the shared input data area.
What is the plan for updating documentation. Could this be done with this PR?
I'm working on this now. I am in the process of setting up the data tar files on our ftp. I am trying a test on cheyenne for a year. One quirky thing I have noticed is that it is ignoring year_init/fyear_init and starting from 2025 when I use year_init = 2005. I assume this is because it is using the istep1 and time from the initial file which is year Jan 1st year 21. Shouldn't the code be ignoring this on an initial run? |
Just found the use_restart_time flag. |
Dumb question. Does year_init have to line up with fyear_init to read in the forcing? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes are working fine here. I would recommend adding the namelist flag:
use_restart_time = .false.
to the test. I am working on setting up the forcing tar files. This will be required if we add the JRA55 set_nml option as a test in Travis-CI.
@rallard77, looks like there are a couple things pending then we can possibly merge this.
While that is going on, I will try to independently download the data from the ftp site and run a test on another machine just to make sure the data is available. Thanks. |
I have updated all of the datasets on our ftp to include the JRA55 forcing. Note that these are really big and the file sizes have increased dramatically. CICE_data_all.tar.gz Also, should we put use_restart_time = .false. in the default ice_in? Does this impact continue runs? Dave |
I don't know much about the use_restart_time flag. Did you need it to get the case to run? By default, it's true and we do not have any tests that change it to false, so we are not exercising it. |
It runs fine. Just that the date of the history and restart files are off by 20 years from year_init. Dave |
I downloaded the "new" CICE tar file to conrad, dropped it in the inputdata area, and ran conrad_intel_restart_gx1_16x2_jra55 successfully. I guess I think we should do a bit more research on the "use_restart_time" flag before switching it for this case. So I guess I feel there are still two minor questions before merging @rallard77
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, though I agree that a few additional things need to be done before the PR is merged:
-
Definitely reinstate the bgc test in the base suite.
-
We do need more documentation about this case, to at least describe what the forcing fields are, what dates they cover, and a citation for the original data set. This can be added in a separate PR but I'd prefer it all go in together.
The calendar related flags in CICE were made to be very flexible, perhaps too flexible -- this is something that can be changed when the time manager is overhauled. Generally speaking, year_init and fyear do not need to be the same, but in this case I think they should be the same. If use_restart_time is true, then I believe it overrides the other flags, except possibly the istep0 value (which I think gets added to whatever the others produce), so it should be false. Note that ycycle=1 indicates that only 1 year of forcing data is being used.
What appears to be going on here is that the new test option mimics the set_nml.gx1 file, which is basically just a smoke test with 24 time steps, and the forcing and simulation dates might not match -- something we hadn't noticed. A full 1- or 5-year test with this data would need additional namelist flags as in set_nml.gx1prod and/or set_nml.qc (but with values appropriate for this data set). It's probably better to set everything appropriately in the set_nml.jra55 file except keep the run short for a smoke test.
- I suggest setting the flags so that the dates on the history and restart files match the dates inherent to the data for longer runs.
This should probably be done for the COREII forcing too, but not in this PR. At some point when we're comfortable with the JRA55 data set, maybe we can switch the QC to this data and retire the COREII data or at least separate it out from the JRA55 download. This will require some longer simulations to fully test and add sample output.
All,
I am at IGS, Let me double check on Monday.
Rick
…On 8/22/2019 1:02 PM, David A. Bailey wrote:
It runs fine. Just that the date of the history and restart files are
off by 20 years from year_init.
Dave
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#350?email_source=notifications&email_token=AG63LB5BB54CCYHZPJZUHXTQF3ICLA5CNFSM4ILYD6L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD455AXQ#issuecomment-524013662>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG63LB4RAJDB36E3DQXIZK3QF3ICLANCNFSM4ILYD6LQ>.
--
logo
*Richard A. Allard *
*Head, Nearshore and Coupled Models Systems Section*
*Oceanography Division NRL Code 7322*
*Stennis Space Center, MS 39529*
*T: 228.688.4894*
[email protected] <mailto:[email protected]>
****
******
|
I support the idea that the setup should be correct for a long run even if we are just carrying out short test runs. |
I provided some additional documentation to "ug_case_settings.rst" but
could easily add more details and a description of JRA55. Where should I
put that?
Rick
…On 8/22/2019 2:54 PM, Tony Craig wrote:
I downloaded the "new" CICE tar file to conrad, dropped it in the
inputdata area, and ran conrad_intel_restart_gx1_16x2_jra55 successfully.
I guess I think we should do a bit more research on the
"use_restart_time" flag before switching it for this case. So I guess
I feel there are still two minor questions before merging @rallard77
<https://github.com/rallard77>
* add the removed test in base_suite.ts if appropriate
* let us know whether the documentation is adequate at this point
and if more is needed whether you propose to defer.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#350?email_source=notifications&email_token=AG63LB3EO25PEICLBF2H4NLQF3VIXA5CNFSM4ILYD6L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD46GUEY#issuecomment-524053011>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG63LB4SULHVJTWCKEQ334LQF3VIXANCNFSM4ILYD6LQ>.
--
logo
*Richard A. Allard *
*Head, Nearshore and Coupled Models Systems Section*
*Oceanography Division NRL Code 7322*
*Stennis Space Center, MS 39529*
*T: 228.688.4894*
[email protected] <mailto:[email protected]>
****
******
|
I just emailed @rallard77 but we also need to update the CICE wiki with information about the new forcing data: Mainly the "Updated Files" section as well as the directory structures and sizes. @dabail10 do you want to handle this since you put them together or should I? |
I can update the wiki. |
Updated the forcing wiki. Let me know if you expected more than this? |
All,
With use_restart_time set to false, year_init and fyear_init set to 2005
(could be any year between 205-2009), the history and restart files all
have dates with 2005 embedded in the file name. I beleive this is what
we want included in the test. One can run a full year with these
settings with a modification to npt.
Rick
…On 8/22/2019 5:38 PM, Elizabeth Hunke wrote:
***@***.**** requested changes on this pull request.
This looks good, though I agree that a few additional things need to
be done before the PR is merged:
*
Definitely reinstate the bgc test in the base suite.
*
We do need more documentation about this case, to at least
describe what the forcing fields are, what dates they cover, and a
citation for the original data set. This can be added in a
separate PR but I'd prefer it all go in together.
The calendar related flags in CICE were made to be very flexible,
perhaps too flexible -- this is something that can be changed when the
time manager is overhauled. Generally speaking, year_init and fyear do
not need to be the same, but in this case I think they should be the
same. If use_restart_time is true, then I believe it overrides the
other flags, except possibly the istep0 value (which I think gets
added to whatever the others produce), so it should be false. Note
that ycycle=1 indicates that only 1 year of forcing data is being used.
What appears to be going on here is that the new test option mimics
the set_nml.gx1 file, which is basically just a smoke test with 24
time steps, and the forcing and simulation dates might not match --
something we hadn't noticed. A full 1- or 5-year test with this data
would need additional namelist flags as in set_nml.gx1prod and/or
set_nml.qc (but with values appropriate for this data set). It's
probably better to set everything appropriately in the set_nml.jra55
file except keep the run short for a smoke test.
* I suggest setting the flags so that the dates on the history and
restart files match the dates inherent to the data for longer runs.
This should probably be done for the COREII forcing too, but not in
this PR. At some point when we're comfortable with the JRA55 data set,
maybe we can switch the QC to this data and retire the COREII data or
at least separate it out from the JRA55 download. This will require
some longer simulations to fully test and add sample output.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#350?email_source=notifications&email_token=AG63LBZ2ZMEUDZG4OUXO7SDQF4IOXA5CNFSM4ILYD6L2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCCOPMAQ#pullrequestreview-278722050>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG63LBYSZQVAE2JZLBYQEQLQF4IOXANCNFSM4ILYD6LQ>.
|
Thanks @mattdturner. I guess I suggest that you create a new option, set_nml.jra55_2008, for the 2008 start date leap year test. I would then setup a test that was "smoke, jra55_2008, 3 months, medium"? That way we clear March 1 for sure. I think this is really mostly a test of the leap year capability with the JRA dataset? With the current test, "smoke gx1 8x1 jra55,short", this is not using the leap year calendar, is that correct? I would modify this to be a restart test and I would increase the pe count (you are running gx1) to something 10x-32x higher? Does that make sense? I haven't quite followed all the details of all the discussions, but my sense is we probably want a restart test here and to be able to run qc if needed. Then separately, we want to have a leap year test with the leap year calendar. Are we thinking that eventually we'll drop the COREII dataset testing? Or are we going to continue to support both? If JRA is going to be our primary forcing dataset, we might want to add bgc gx1 tests with JRA forcing too. One final issue I'm noticing, the set_nml.gx1 sets the COREII forcing dataset then set_nml.jra55 switches that to JRA55. Ideally, we'd clean this up a bit, maybe renaming gx1 to gx1_coreii and adding gx1_jra55. or maybe removing the coreii stuff from set_nml.gx1 and then adding a set_nml.coreii. But I think we can defer this for the time being for backwards compatibility. I assume you've confirmed that by using gx1 and jra55, the tests are being setup with the jra forcing correctly. |
…so add a restart test for JRA55 in base_suite
Correct, the smoke test is basically just to test the
Yes
My understanding is that, eventually, JRA55 would replace COREII. I could be wrong though. I've pushed changes that add a new jra55_2008 namelist option, and a new run90day option. It also changes the tests in base_suite.ts to do a 90 day smoke test for jra55_2008, and a JRA55 smoke test. I am currently running |
So far, all tests have passed. There are still 2 tests that I am waiting for ( I don't think any documentation needs to be updated for the recent changes I made. I have no idea if the JRA55 documentation is sufficient (I'm not suggesting that it isn't, either). Somebody more familiar with JRA55 would need to make that conclusion. |
@@ -338,6 +338,7 @@ Table of namelist options | |||
"","``restore_ice``", "true/false", "restore ice state along lateral boundaries", "" | |||
"\*","``atm_data_type``", "``default``", "constant values defined in the code", "" | |||
"","", "``LYq``", "AOMIP/Large-Yeager forcing data", "" | |||
"","", "``JRA55``", "JRA55 forcing data", "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add the citation here. You want to do it with the following format :cite:Tsujino18
(see below for box2001 as an example).
You will also have to add the citation at the very en of the following file (right after Roberts18: /doc/source/master_list.bib
@Article{Tsujino18,
author = "H. Tsujino and S. Urakawa and R.J. Small and W.M. Kim and S.G. Yeager and et al.",
title = "{JRA‐55 based surface dataset for driving ocean–sea‐ice models (JRA55‐do)}",
journal = OM,
year = {2018},
url = {http://dx.doi.org/10.1016/j.ocemod.2018.07.002}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the citation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments in ug_case_settings.rst
@rallard77 @apcraig @eclare108213 @dabail10 I have gone through the CICE Wiki (https://github.com/CICE-Consortium/CICE/wiki/CICE-Input-Data) to add the JRA55 information in the Input Data section. Please check that the citations are correct for JRA55 and COREII. Additionally, I noticed that the yearly forcing didn't include JRA55, so I'm remaking the forcing files and will put them in the FTP asap. I also removed the other "new" files since they were from a year ago and we say on the wiki that after 6 months we no longer consider the data to be "new". Otherwise I've worked with @rallard77 on the documentation through the repo and minus a few small details we should be ready to go here soon. |
I just updated the wiki pages with the individual year files and sizes of files. I think we're all good to go on that end of the documentation. Let me know if anyone sees an issue there. So just the addition of the citation into the rst files in the main repo. |
The citations should have been added to the rst files already. So it sounds like this PR might be good to go |
@mattdturner Yep, I see them in there now. Looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had another look at the code modifications and I don't have any concerns at this point. Unless there are outstanding issues still to be done that I'm not aware of or @eclare108213 has some comments, I think we can merge.
This looks fine to me except for the conflicting file. |
@mattdturner it looks like the conflict is with the updated documentation file. This is one I changed yesterday to add the reference for COREII. I think if you just do a "git pull upstream master" to the development fork (assuming the master branch is your upstream) that will take care of it. |
I just edited the conflicted doc file in the PR. Sometimes you can do that with a conflict. I committed the change thru the github interface. That correction is now being checked with travis and readthedocs. Once those both pass, we should be able to merge. |
@apcraig It looks like the documentation build is good, so as soon as Travis passes I think we can merge. |
Sure,
I will be able to put together some plots....
Rick
…On 9/19/2019 11:10 AM, Elizabeth Hunke wrote:
This looks fine to me except for the conflicting file.
Not for this PR but for the release, can one of you who has done a run
with this data set (preferably using a standard test from the test
suite) make some plots similar to those with COREII, for the sample
output wiki page? Thanks everyone! This is a big step for us.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#350?email_source=notifications&email_token=AG63LBZCOXLEFGL53EIU3ALQKOP7BA5CNFSM4ILYD6L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7EAJ7Q#issuecomment-533202174>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG63LBYLZDKPTX32THSR5ETQKOP7BANCNFSM4ILYD6LQ>.
|
All,
Attached are a few slides for consideration for the wiki. The first is
taken from the wiki and slides 2 and 3 are my suggested contributions.
We ran a full year using JRA55 with gx1 for 2008. The actual test in the
base_suite runs for 60 days in 2008 through March 1. The users should
replicate the March 1 results. If they run for a full year, they should
also get the same results for Sept 1. I can change these slides based on
feedback.
Rick
…On 9/19/2019 11:10 AM, Elizabeth Hunke wrote:
This looks fine to me except for the conflicting file.
Not for this PR but for the release, can one of you who has done a run
with this data set (preferably using a standard test from the test
suite) make some plots similar to those with COREII, for the sample
output wiki page? Thanks everyone! This is a big step for us.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#350?email_source=notifications&email_token=AG63LBZCOXLEFGL53EIU3ALQKOP7BA5CNFSM4ILYD6L2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7EAJ7Q#issuecomment-533202174>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG63LBYLZDKPTX32THSR5ETQKOP7BANCNFSM4ILYD6LQ>.
|
@rallard77 I don't see attached plots. Maybe email them directly to me? |
Or drag and drop into a comment box directly on this github page. |
…c forcing on gx1 grid.
For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
PR checklist
Added new capability to perform tests with JRA55 atmospheric forcing on gx1 grid for years 2005-2009.
Richard Allard, Julie Crout, Matt Turner
repo = https://github.com/rallard77/CICE.git
#bran = jra55
#hash = eff67a2
#hshs = eff67a2
#hshu = Elizabeth Hunke [email protected]
#hshd = Thu Aug 1 17:36:22 2019 -0600
#date = 2019-08-13
#time = 14:28:39
#mach = conrad
#user = allard
#vers = CICE_6.0.1
...skipping most of the log file
PASS conrad_gnu_restart_gx1_8x1_jra55 build
PASS conrad_gnu_restart_gx1_8x1_jra55 initialrun
PASS conrad_gnu_restart_gx1_8x1_jra55 run 94.60 16.34 47.64
PASS conrad_gnu_restart_gx1_8x1_jra55 test
#---
COPY conrad_gnu_smoke_gx3_1x1_diag1_run1day build
PASS conrad_gnu_smoke_gx3_1x1_diag1_run1day run 9.00 1.52 4.49
PASS conrad_gnu_smoke_gx3_1x1_diag1_run1day test
#---
COPY conrad_gnu_restart_gbox128_8x1_diag1 build
PASS conrad_gnu_restart_gbox128_8x1_diag1 initialrun
PASS conrad_gnu_restart_gbox128_8x1_diag1 run 30.07 8.70 15.00
PASS conrad_gnu_restart_gbox128_8x1_diag1 test
#---
COPY conrad_gnu_restart_gx3_4x2_debug_diag1_run5day build
PASS conrad_gnu_restart_gx3_4x2_debug_diag1_run5day initialrun
PASS conrad_gnu_restart_gx3_4x2_debug_diag1_run5day run 50.30 22.29 14.04
PASS conrad_gnu_restart_gx3_4x2_debug_diag1_run5day test
#-------
#totl = 708
#pass = 708
#fail = 0
#pend = 0
2 new subroutines added to ice_forcing.F90: JRA55_files and JRA55_data. In Ice-in, atm_data_type is set to "JRA55" to use this option. I have 5 .nc files for each year of forcing (2005-2009). Unlike COREII, we use shortwave and do not need a cloud factor. Forcing is at 3-hr intervals for all needed fields including precipitation. Testing was performed on conrad XC40 using quick_suite and base_suite. All tests passed. 1 new test case added to base_suite.ts which runs a 10-day restart with JRA55 for 2005. Added abort syntax in JRA55_data to ensure that days_per_year is set to 366 for leap year. The data is expected to be located in ./gx1/forcing/JRA55